Skip to content

fix(disclosure): persist attempt before irreversible delivery - #721

Merged
imran-siddique merged 6 commits into
agentrust-io:mainfrom
altrudev:fix/660-durable-disclosure-attempt
Oct 6, 2026
Merged

imran-siddique merged 6 commits into
agentrust-io:mainfrom
altrudev:fix/660-durable-disclosure-attempt

Conversation

@altrudev

@altrudev altrudev commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the local crash-window evidence gap identified in #660 around irreversible disclosure delivery.

This keeps the existing ordering:

verify -> consume request ID -> recheck validity

and adds:

persist minimized attempt(unknown) -> deliver exact bytes -> best-effort durable acknowledge

Behavior

  • persists a minimized disclosure-attempt row before recipient.deliver()
  • fails closed if that pre-delivery audit write fails
  • keeps delivery unknown if delivery is interrupted or the post-delivery audit update fails
  • upgrades the same event to acknowledged only after normal callback return
  • stores only event_id, disposition, reason, and delivery
  • does not store payload, payload digest, request ID, principal, recipient, purpose, source scope, labels, or approval

Tests

Added restart and storage-failure coverage, including:

  • durable unknown record across callback failure/reopen
  • durable acknowledged record across reopen
  • BaseException interruption leaves unknown
  • pre-delivery audit failure prevents delivery
  • post-delivery audit failure retains unknown
  • audit schema contains only minimized fields

Validation run from exact upstream baseline 4acb813487375578098cd4a23620ecdd647bc6cf:

  • focused disclosure/sink gate: 82 passed
  • full suite: 2410 passed, 37 skipped
  • git diff --check: clean

This does not change the release authorization model, confinement claims, downstream execution claims, or disclosure claim ceilings.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

🔴 Contributor Check: HIGH

Check Result
Profile HIGH
Credential LOW
Overall HIGH

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:HIGH Contributor check flagged HIGH risk label Oct 3, 2026
@codecov-commenter

codecov-commenter commented Oct 3, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 86.44068% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cmcp_runtime/disclosure.py 86.44% 8 Missing ⚠️

📢 Thoughts on this report? Let us know!

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recheck approval validity after the audit write, immediately before delivery, and add a regression covering expiry during that write.

@altrudev

altrudev commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the requested pre-delivery validity recheck. The gate now revalidates a scoped approval after the durable audit write and immediately before the irreversible delivery callback. Added a regression where the first two validity checks succeed and the approval expires during the audit-write window; delivery remains unattempted and the one-use request stays consumed. Updated the disclosure contract text accordingly. Verification: targeted disclosure/sink tests 83 passed; ruff clean; mypy clean; full suite 2411 passed, 37 skipped (environment-dependent), with only existing Python ctypes deprecation warnings.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@altrudev the recheck is right. One consequence of where it sits: when it fails, the attempt row is already durable as delivery_attempted/unknown and stays that way, so the store reports a possible disclosure that was never attempted, and the returned denial has no event_id to tie it to that row. Downgrade the row best-effort to not attempted on that path, the way acknowledge upgrades it on success, keep unknown if that write fails, and return the event_id. Extend your expiry regression to assert the stored row. Can you have it up by 9 October?

altrudev commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the second review through a bounded DDC/Frequency pass on exact head ba3b827003852587dea1485ad1df89e2b4ecbb0e.

The final pre-delivery validity failure now reconciles the already-durable prepared event instead of leaving a false possible-disclosure signal:

  • best-effort updates the same audit row to the denial disposition/reason with delivery=not_attempted;
  • returns the same event_id in the denial;
  • if that corrective write itself fails, the durable row remains conservatively delivery_attempted/unknown and the returned denial carries that same event_id with delivery=unknown;
  • the request ID remains consumed in both cases, so there is no silent retry path;
  • the expiry regression now asserts the stored row, and an additional regression covers corrective-write failure.

The disclosure contract text was updated to match that state machine.

Verification from a fresh checkout on the current head:

  • focused disclosure + sink policy: 84 passed
  • Ruff: pass
  • mypy (src/cmcp_runtime): pass, 63 source files
  • git diff --check: pass
  • full suite: 2412 passed, 37 skipped, 4 existing ctypes deprecation warnings

Claim boundary is unchanged: not_attempted is only recorded after the final admission recheck prevents dispatch; unknown is retained whenever the corrective durable state transition cannot be established.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final recheck now leaves the row as not_attempted with the denial's disposition and returns its event_id, and keeps unknown when the corrective write fails. Both paths are pinned by tests. Approving.

@imran-siddique
imran-siddique merged commit 3743f74 into agentrust-io:main Oct 6, 2026
27 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:HIGH Contributor check flagged HIGH risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants